Node / meshtastic / Meshtastic-Android / files / docs / decisions / architecture-review-2026-03.md

Displaying Rendered • View raw • Download

│ Status: Active Last updated: 2026-03-31

Re-evaluation of project modularity and architecture against modern KMP and Android best practices. Identifies gaps and actionable improvements across modularity, reusability, clean abstractions, DI, and testing.

The codebase is ~98% structurally KMP — 18/20 core modules and 8/8 feature modules declare T383838jvm() targets and cross-compile in CI. Shared T383838commonMain code accounts for ~52K LOC vs ~18K platform-specific LOC (a 74/26 split). This is strong.

Of the five structural gaps originally identified, four are resolved and one remains in progress:

1. T383838app is a God module — originally 90 files / ~11K LOC of transport, service, UI, and ViewModel code that should live in core/feature modules. (✅ Resolved — app module reduced to 6 files: T383838MainActivity, T383838MeshUtilApplication, Nav shell, and DI config)

2. ~~Radio transport layer is app-locked~~ — ✅ Resolved: T383838RadioTransport interface in T383838core:repository/commonMain; shared T383838StreamFrameCodec + T383838TcpTransport in T383838core:network.

3. ~~T383838java.* APIs leak into T383838commonMain~~ — ✅ Resolved: T383838Locale, T383838ConcurrentHashMap, T383838ReentrantLock purged.

4. ~~Zero feature-level T383838commonTest~~ — ✅ Resolved: 193 shared tests across all 8 features; T383838core:testing module established.

5. ~~No T383838feature:connections module~~ — ✅ Resolved: KMP module with shared UI and dynamic transport detection.

Key ratio: 74% of production code is in T383838commonMain (shared). Goal: 85%+.

The T383838app module should be a thin shell (~20 files): T383838MainActivity, DI assembly, nav host. Originally it held 90 files / ~11K LOC, now completely reduced to a 6-file shell:

Progress: Extracted T383838ChannelViewModel → T383838feature:settings/commonMain, T383838NodeMapViewModel → T383838feature:map/commonMain, T383838NodeContextMenu → T383838feature:node/commonMain, T383838EmptyDetailPlaceholder → T383838core:ui/commonMain. Remaining extractions require radio/service layer refactoring (bigger scope).

The core transport abstraction was previously locked in T383838app/repository/radio/ via T383838IRadioInterface. This has been successfully refactored:

1. Defined T383838RadioTransport interface in T383838core:repository/commonMain (replacing T383838IRadioInterface)

2. Moved T383838StreamFrameCodec-based framing to T383838core:network/commonMain

3. Moved TCP transport to T383838core:network/jvmAndroidMain

4. The remaining T383838app/repository/radio/ implementations (BLE, Serial, Mock) now implement T383838RadioTransport.

1. Move BLE transport to T383838core:ble/androidMain

2. Move Serial/USB transport to T383838core:service/androidMain

Device discovery UI was duplicated:

• Android: T383838app/ui/connections/ (13 files: T383838ConnectionsScreen, T383838ScannerViewModel, 10 components)

• Desktop: T383838desktop/ui/connections/DesktopConnectionsScreen.kt (separate implementation)

Outcome: Created T383838feature:connections KMP module with:

• T383838commonMain: T383838ScannerViewModel, T383838ConnectionsScreen, 11 shared UI components, T383838DeviceListEntry sealed class, T383838GetDiscoveredDevicesUseCase interface, T383838CommonGetDiscoveredDevicesUseCase (TCP/recent devices)

• T383838androidMain: T383838AndroidScannerViewModel (BLE bonding, USB permissions), T383838AndroidGetDiscoveredDevicesUseCase (BLE/NSD/USB discovery), T383838NetworkRepository, T383838UsbRepository, T383838SerialConnection

• Desktop uses the shared T383838ConnectionsScreen + T383838CommonGetDiscoveredDevicesUseCase directly

• Dynamic transport detection via T383838RadioInterfaceService.supportedDeviceTypes

• Module registered in both T383838AppKoinModule and T383838DesktopKoinModule

T383838core:api is Android-only (AIDL IPC). T383838ServiceClient in T383838core:service/androidMain wraps it. Desktop doesn't use it — it has T383838DirectRadioControllerImpl in T383838core:service/commonMain.

Recommendation: The T383838DirectRadioControllerImpl pattern is correct. Ensure T383838RadioController (already in T383838core:model/commonMain) is the canonical interface; deprecate the AIDL-based path for in-process usage.

Outcome: The three T383838Locale usages identified in March were removed from T383838commonMain. Follow-up cleanup in the same sprint also moved T383838ReentrantLock-based T383838SyncContinuation to T383838jvmAndroidMain, replaced prefs T383838ConcurrentHashMap caches with atomic persistent maps, and pushed enum reflection behind T383838expect/T383838actual so no known T383838java.* runtime calls remain in T383838commonMain.

Formerly found in 3 prefs files:

• T383838core:prefs/.../MeshPrefsImpl.kt

• T383838core:prefs/.../UiPrefsImpl.kt

• T383838core:prefs/.../MapConsentPrefsImpl.kt

Outcome: These caches now use T383838AtomicRef<PersistentMap<...>> helpers in T383838commonMain, eliminating the last T383838ConcurrentHashMap usage from shared prefs code.

T383838MQTTRepositoryImpl has been migrated to T383838commonMain using KMQTT, replacing Eclipse Paho.

Fix: Completed.

• T383838kmqtt library integrated for full KMP support.

Vico chart screens (DeviceMetrics, EnvironmentMetrics, SignalMetrics, PowerMetrics, PaxMetrics) have been migrated to T383838feature:node/commonMain using Vico's KMP artifacts (T383838vico-compose, T383838vico-compose-m3). Desktop wires them via shared composables. No Android-only chart code remains.

Comprehensive audit of T383838androidMain vs T383838jvmMain duplication across all feature modules. Extracted shared components:

Also fixed T383838Dispatchers.IO usage in T383838StoreForwardPacketHandlerImpl (would break iOS), removed dead T383838UIViewModel.currentAlert property, and added T383838firebase-debug.log to T383838.gitignore.

T383838DesktopKoinModule.kt originally had ~120 lines of hand-written T383838viewModel { ... } blocks. These have been successfully replaced by including Koin modules from T383838commonMain generated via the Koin K2 Compiler Plugin for automatic wiring.

T383838desktopPlatformStubsModule() previously had stubs that were only validated at runtime.

Outcome: Added T383838DesktopKoinTest.kt using Koin's T383838verify() API. This test validates the entire Desktop DI graph (including platform stubs and DataStores) during the build. Discovered and fixed missing stubs for T383838CompassHeadingProvider, T383838PhoneLocationProvider, and T383838MagneticFieldProvider.

Android uses T383838@Module-annotated classes (T383838CoreDataModule, T383838CoreBleAndroidModule). Desktop imports them as T383838CoreDataModule().coreDataModule(). This works but the double-invocation pattern is non-obvious.

Recommendation: Document the pattern in AGENTS.md. Consider if Koin Annotations 2.x supports a simpler import syntax.

Outcome: All 8 feature modules now have T383838commonTest coverage (193 shared tests). Combined with 70 platform unit tests and 18 instrumented tests, feature modules have 281 tests total.

T383838core:testing module established with shared fakes (T383838FakeNodeRepository, T383838FakeServiceRepository, T383838FakeRadioController, T383838FakePacketRepository) and T383838TestDataFactory builders. Used by all feature T383838commonTest suites.

36 T383838commonTest files exist but are concentrated in T383838core:domain (22 files) and T383838core:data (10 files). Limited or zero tests in:

• T383838core:service (has T383838ServiceRepositoryImpl, T383838DirectRadioControllerImpl, T383838MeshServiceOrchestrator)

• T383838core:network (has T383838StreamFrameCodecTest — 10 tests; T383838TcpTransport untested)

• T383838core:prefs (preference flows, default values)

• T383838core:ble (connection state machine)

• T383838core:ui (utility functions)

T383838desktop/src/test/ contains T383838DesktopKoinTest.kt and T383838DesktopTopLevelDestinationParityTest.kt. Still needs:

• Navigation graph coverage

Ordered by impact × effort:

Room KMP's T383838setAutoCloseTimeout API is Android-only. On JVM/Desktop, once a Room database is built, its SQLite connections (5 per WAL-mode DB: 4 readers + 1 writer) remain open indefinitely until explicitly closed via T383838RoomDatabase.close().

When a user switches between multiple mesh devices, the previous device's database remained open in the in-memory cache. Each idle database consumed ~32 MB (connection pool + prepared statement caches), leading to unbounded memory growth proportional to the number of devices ever connected in a session.

T383838DatabaseManager.switchActiveDatabase() now explicitly closes the previously active database via T383838closeCachedDatabase() before activating the new one. The closed database is removed from the in-memory cache but its file is preserved, allowing transparent re-opening on next access.

Additional fixes applied:

1. Init-order bug: T383838dbCache was declared after T383838currentDb, causing NPE during T383838stateIn's T383838initialValue evaluation. Reordered to ensure T383838dbCache is initialized first.

2. Corruption handlers: T383838ReplaceFileCorruptionHandler added to T383838createDatabaseDataStore() on both JVM and Android, preventing DataStore corruption from crashing the app.

3. T383838desktopDataDir() deduplication: Made public in T383838core:database/jvmMain and removed the duplicate from T383838DesktopPlatformModule, establishing a single source of truth for the desktop data directory.

4. DataStore scope consolidation: Replaced two separate T383838CoroutineScope instances with a single shared T383838dataStoreScope in T383838DesktopPlatformModule.

5. Coil cache path: Desktop T383838Main.kt updated to use T383838desktopDataDir() instead of hardcoded T383838user.home.

• Current migration status: kmp-status.md

• Roadmap: roadmap.md

• Agent guide: ../AGENTS.md

• Decision records: decisions/